Skip to content

operator: exempt stuck pods' claims from the PVCUnbinder pvc-rebinding gate#1658

Open
RafalKorepta wants to merge 1 commit into
mainfrom
rk/pvcunbinder-gate3-wffc-deadlock
Open

operator: exempt stuck pods' claims from the PVCUnbinder pvc-rebinding gate#1658
RafalKorepta wants to merge 1 commit into
mainfrom
rk/pvcunbinder-gate3-wffc-deadlock

Conversation

@RafalKorepta

@RafalKorepta RafalKorepta commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Problem

The PVCUnbinder's Gate 3 (pvc-rebinding) defers any unbind while any claim in the cluster has an empty spec.volumeName, assuming an in-progress rebind that will settle shortly. That assumption is false for claims owned by Pods that are themselves stuck, producing a three-way circular wait that deadlocks the unbinder:

  1. Gate 3 defers because a stuck Pod's datadir claim has no volumeName,
  2. that claim can never bind until its Pod schedules (WaitForFirstConsumer), and
  3. the Pod can never schedule until the unbinder frees its mis-pinned sibling claim — the very action Gate 3 is deferring.

Production impact (cluster d99mur9p5l53r2240m3g, 2026-07-12): a fresh AWS cluster never bootstrapped. broker-1's shadow-index-cache PV was provisioned onto broker-0's node; the pod could never fit there (didn't have free ports) nor anywhere else (didn't match PersistentVolume's node affinity). The unbinder logged a PVC in this cluster has no volumeName yet; deferring every 30s for ~2h while brokers 0/2 looped in cluster_discovery waiting for the missing seed. Manual PVC deletion was required.

Fix

Exempt claims owned by Pods stuck Pending with the unbinder's scheduling-failure signature (schedulingFailureRE — the same weak form ShouldRemediate accepts), the reconciled Pod first among them. Same reasoning as Gate 0's own-claim exemption (inFlightClaimOwnedBy), which exists for exactly this deadlock class.

  • Symmetric across victims: two simultaneously mis-pinned brokers don't mutually defer on each other's unbound claims (adversarial review of the initial own-claims-only version found and empirically reproduced that variant). Destructive work stays serialized by Gate 0.
  • Deliberately weak signature: modern schedulers prefix every total scheduling failure with 0/N nodes are available: and don't reliably name volume affinity, so a mid-rebind Pod can also be exempted. Safe with rebinding off: unbinder-initiated rebinds still defer siblings through Gate 0's uncached settle check, the destructive pipeline only touches the reconciled Pod's own claims, and no Available PV is ever produced for a floating claim to mis-pair with.
  • --allow-pv-rebinding keeps the old conservative behavior (no exemption): that mode floats freed PVs as live binding candidates (see FreedPVAnnotation / INC-2818), where acting while any claim is unbound could pair it with a disk it was never meant to hold.
  • Orphaned claims (no Pod) keep deferring — conservative, alertable via the gate metric.

Also:

  • The Gate 3 log line and Event now name the gating claim (attribution of the production incident took hours partly because the log never said which claim was unbound).
  • Updated the stale PVCUnbinderGateDeferred metric doc comment.
  • Fixed the withPVC test fixture to produce unique volume names (previously two calls produced duplicate name: data volumes — invalid against a real API server).

Testing

TDD; each new subtest was watched failing against the old code first:

  • own unbound WFFC claim does not deadlock the unbind — the production scenario end-to-end through Reconcile (mis-pinned bound claim deleted, PV Retain + in-flight annotations, pod deleted, unbound claim untouched).
  • two stuck pods with unbound WFFC claims do not mutually deadlock — the ≥2-victim variant.
  • allow-pv-rebinding keeps the conservative deferral even for own claims.
  • sibling stuck on a non-affinity total scheduling failure is also exempted (weak signature) — pins the deliberate breadth.
  • sibling unbound claim still defers — pins serialization, and asserts via events.FakeRecorder that the pvc-rebinding gate specifically fired and the event names the gating claim.

go test ./internal/controller/pvcunbinder/ ./internal/observability/ green; gofmt/go vet clean.

The change was additionally put through a multi-agent adversarial review (10 finder angles → per-finding verification → sweep); all confirmed correctness findings are addressed here or listed below.

Known pre-existing limitations (out of scope, surfaced by the review)

  • An orphaned never-bound claim (aborted scale-up, then scale-in — STS scale-in retains PVCs) still defers all unbinds in its cluster forever.
  • The same mis-provisioning class deadlocks at Gate 2 (multi-node) instead when two victims' PVs land on two different occupied nodes: Gate 2 classifies it as a K8s-wide event and defers to a natural recovery that WFFC mis-provisioning doesn't have.

🤖 Generated with Claude Code

K8S-901

@secpanda

secpanda commented Jul 12, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

//
// Names are unambiguous without a namespace: both this scan and
// [Controller.listClusterPVCsByName] are scoped to pod.Namespace.
func (r *Controller) stuckClaimNames(ctx context.Context, pod *corev1.Pod) (map[string]struct{}, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having a hard time understanding what this function does:

  1. We get all PVCs for given pod argument (regardless of their status / being stuck) (from StsPVCs(pod))
  2. Then we list all the pods for given Redpanda cluster and we add all stuck PVCs for these pods?
    Is this correct?

@RafalKorepta RafalKorepta force-pushed the rk/pvcunbinder-gate3-wffc-deadlock branch from 941b6e1 to bf5d857 Compare July 13, 2026 12:37
@RafalKorepta RafalKorepta reopened this Jul 14, 2026
@RafalKorepta RafalKorepta force-pushed the rk/pvcunbinder-gate3-wffc-deadlock branch from bf5d857 to b072c73 Compare July 16, 2026 13:28
…g gate

Gate 3 (pvc-rebinding) defers any unbind while a claim in the cluster
is unbound, assuming an in-progress rebind that will settle shortly.
That assumption is false for claims owned by Pods that are themselves
stuck Pending on a scheduling failure, and deadlocks the unbinder in a
three-way circular wait:

  1. Gate 3 defers because a stuck Pod's datadir claim has no
     volumeName,
  2. that claim can never bind until its Pod schedules
     (WaitForFirstConsumer), and
  3. the Pod can never schedule until the unbinder frees its
     mis-pinned sibling claim (e.g. a shadow-index-cache PV
     provisioned onto an already-occupied node) — the very action
     Gate 3 is deferring.

Hit in production by a fresh AWS cluster that never bootstrapped:
broker-1's shadow-index-cache PV landed on broker-0's node, the pod
could never fit there ("didn't have free ports") nor anywhere else
("didn't match PersistentVolume's node affinity"), and the unbinder
logged "a PVC in this cluster has no volumeName yet; deferring" every
30s for hours while the remaining brokers looped in cluster discovery
waiting for the missing seed.

Exempt a claim from Gate 3's deferral only under a full proof chain,
evaluated identically for the reconciled Pod and each sibling (so two
simultaneously mis-pinned brokers don't mutually defer on each
other's unbound claims, while destructive work stays serialized by
Gate 0):

  - the Pod passes ShouldRemediate in full — Selector, StatefulSet
    ownership, Pending, the weak scheduling-failure signature (modern
    schedulers don't reliably name volume affinity in the message),
    and the unbind Timeout measured against the Unschedulable
    condition's LastTransitionTime;
  - the Pod holds a Bound claim on a HostPath/Local PV — bound is
    proven by the PV's ClaimRef back-reference (namespace, name, and
    UID all matching the claim), never by the claim's user-settable
    volumeName alone — whose NodeAffinity-eligible node set (every hostname the selector
    accepts, resolved by the kubernetes.io/hostname LABEL rather than
    the Node object name; unresolvable selector shapes fail closed)
    is entirely unavailable: node gone, cordoned, or
    NotReady/unreachable — with the Ready condition and BOTH effect
    twins of the not-ready/unreachable taints (the node lifecycle
    controller applies NoExecute and NoSchedule together) judged
    through one canonical NoExecute toleration lens, so the
    auto-injected TolerationSeconds grace tolerations don't count
    while tolerate-forever pods
    (--broker-pod-node-unavailable-toleration=-1s, whose contract is
    that only Node deletion signals permanent loss) never treat
    transient unreachability as proof — or occupied by a live pod
    matching one of the Pod's own RequiredDuringScheduling
    anti-affinity terms (only terms whose full semantics are
    interpretable: hostname topology, own-namespace scope including
    the v1 Cluster's explicit single-namespace shape, no
    matchLabelKeys — anything else fails closed; candidate occupants
    are every pod in the namespace, so a custom term that selects a
    different workload still proves its occupant);
  - only the Pod's own unbound claims that would bind under a
    WaitForFirstConsumer StorageClass are exempted, with the class
    resolved exactly as Kubernetes' GetPersistentVolumeClaimClass
    resolves it (beta annotation first, by key presence, then
    spec.storageClassName; no fallback to the cluster's current
    default).

Every read that can grant an exemption goes through the uncached
APIReader — the reconciled Pod itself (re-qualified via ShouldRemediate
on an uncached re-read decoded into a fresh object — decoding into
the cache-populated one would merge and keep stale fields the fresh
response omits — before any evidence or destructive step, so a
stale informer copy of a since-resolved Pod can neither supply
exemption evidence nor reach the PVC deletes), node state, occupant
pods, sibling discovery, the per-claim PVC evidence Gets, and the
StorageClass binding-mode Get (the sole exception, the mis-pinned PV
Get, only consults fields that never change on a live Bound PV) —
so a lagging informer can neither fabricate a mis-pin nor keep an
already-resolved sibling looking stuck. Deferral-only reads stay
cached. The node/occupant/PVC/StorageClass evidence chain is computed
lazily, only when some claim is actually unbound, so the common
all-claims-bound remediation path adds only the one uncached Pod
re-read (which runs on every qualifying reconcile, disable flag or
not); evidence-read failures (e.g. a 403 on
the nodes LIST under RBAC version skew) fall back fail-safe to the
conservative deferral with the gate metric and Event instead of
failing the reconcile (context cancellation still surfaces as an
error); and even with every unbound claim exempted, the reconciled
Pod must additionally hold its own mis-pin proof before deletion
proceeds — a sibling's deadlock never transitively authorizes
destroying a Pod that isn't part of one. Exemption-based gate passes
leave the same paper trail deferrals do (the
pvc_unbinder_gate_exempted_total metric, a Warning
PVCUnbinderGateExempted Event naming the exempted claims — capped by
name count and total rendered length so the note never exceeds the
events.k8s.io/v1 1024-char limit, past which the broadcaster
silently drops the Event — and a log line carrying the full list), recorded only when the reconcile
actually proceeds — a
reconcile still deferred by the durable freed-pv gate does not count
gate passes every 30s — deferral messages
name a deterministic (sorted-first) gating claim, siblings owning
none of the unbound claims are skipped without an evidence run, and
claims with no Pod at all keep deferring. A
--disable-pvc-rebinding-gate-exemption flag (operator run/multicluster
and sidecar binaries; exposed in the redpanda chart as
statefulset.sideCars.pvcUnbinder.disableStuckClaimExemption) turns
just the exemption off as an escape hatch, keeping the rest of the
unbinder running.

Under the deprecated --allow-pv-rebinding flag no exemption applies:
that mode floats freed PVs as live binding candidates (see
FreedPVAnnotation / INC-2818), where acting while any claim is unbound
could pair it with a disk it was never meant to hold, so the original
conservative deferral is kept there. The freed-pv gate resolves a
freed PV's pinned node by the kubernetes.io/hostname label too — a
name-based Get would report a live node as gone under kubelet
--hostname-override and silently disengage the gate.

Also name the gating claim in the Gate 3 log line and Event (the
production incident took hours to attribute partly because the log
never said WHICH claim was unbound), update the stale
PVCUnbinderGateDeferred metric doc, grant the unbinder storageclasses
get and nodes list, fix the withPVC test fixture to produce unique
volume names, and rewrite pvcunbinder.go's pre-existing doc comments
file-wide into substantially shorter plain-English sentences (same
invariants).

Known limitations, out of scope here: an orphaned never-bound claim
(aborted scale-up, then scale-in) still defers all unbinds in its
cluster; deployments whose pod-anti-affinity yields no interpretable
required hostname-topology term (type soft, or hard/custom terms of
any other shape — terms are judged by shape, not by which chart
option produced them) get no occupancy-based exemption (Gate 3 keeps
deferring, alertable via the gate metric); and the same
mis-provisioning class deadlocks at Gate 2
instead when two victims' PVs land on two different occupied nodes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XG2iDDtCFzz4QSo3FMsAyR
@RafalKorepta RafalKorepta force-pushed the rk/pvcunbinder-gate3-wffc-deadlock branch from b072c73 to 8080bb2 Compare July 16, 2026 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants